Stop using runtime C++ exceptions#145
Stop using runtime C++ exceptions#145maelhos wants to merge 2 commits intoDmitrySoshnikov:masterfrom
Conversation
As of LLVM 18, llvm-config contains -fno-exceptions, general standard are also moving off runtime exceptions in C++ https://stackoverflow.com/questions/4506369/when-and-how-should-i-use-exception-handling .
|
|
||
| std::cerr << errMsg.str(); | ||
| throw new std::runtime_error(errMsg.str().c_str()); | ||
| std::exit(1); |
There was a problem hiding this comment.
Should this be EXIT_FAILURE as well?
|
@maelhos, thanks for the contribution! Hopefully this is not a breaking change anywhere (except the cases which may catch for the specific exceptions, but that should be fine). Can you confirm the |
|
For that you would need stdlib.h, it doesn't seem to be included in this part of the template though it probably is in the final header, I used 1 to play it safe but if you have the ability to test if EXIT_FAILURE works that would be great. |
|
You can add it here: https://github.com/DmitrySoshnikov/syntax/blob/master/src/plugins/cpp/templates/lr.template.h#L23 Where you able to test the generated code? I'll try looking at #141 later if you're blocked on testing, unless you reach that issue earlier. |
|
I dont't think it is possible to test generation without solving #141 since it happens at compile time though the package syntax-cli available through npm still works fine. |
|
Yes, good idea - we can probably change it in the follow up PR. For this one - can you update please to add |
|
Actually, a program should be able to handle parse error at runtime. With the full exit this now changes semantics, right? - you can't catch the parse errors anymore? |
2 participants
Replace
throwbystd::exitAs of LLVM 18, llvm-config --cxxflags contains -fno-exceptions, general standards are also moving out runtime exceptions in C++ as much as possible c.f. stackoverflow discussion.
I can't run test since the dev is broken because of transition from babel 6 to 7 (I don't know enough about babel to update everything to babel 7) and there is no version nor instruction specifying the required npm and node.